Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Azure pipelines #373

Closed
wants to merge 6 commits into from
Closed

Azure pipelines #373

wants to merge 6 commits into from

Conversation

vhatsura
Copy link
Contributor

@vhatsura vhatsura commented May 9, 2019

This PR adds support for running cake.recipe on AzurePipelines with all main features. (another part of #259)

Test run on AzurePipelines you can check here, which is built from custom branch in fork

Changes:

  • type of Number property of IBuildInfo interface was changed from int to string (BuildNumber on AzurePipelines is string)
  • IBuildProvider interface is extended by UploadArtifact(FilePath file) method
  • there is implementation of IBuildProvider for AzurePipelines
  • UploadAppVeyorArtifactsTask renamed to UploadArtifactsTask ("Upload-AppVeyor-Artifacts" -> "Upload-Artifacts")

Open questions:

  • The main task for running on CI is called AppVeyor. I didn't rename it due to backward compatibility. What we need to do it with it, because if I want to run my cake on AzurePipelines with Cake.Recipe I need to call AppVeyour target, which is a litle bit confusing.

@gep13
Copy link
Member

gep13 commented May 9, 2019

@vhatsura thank you for submitting this PR, I really appreciate it. Just to make you aware, my main focus just now is updating current known uses of Cake.Recipe (mainly in the Cake-Contrib Organisation) to use a pinned version of Cake.Recipe so that any incoming breaking changes don't affect any upstream callers who aren't pinned to the "released" version.

Once that is done, I plan on looking at merging PR's.

Thanks for your understanding.

Copy link
Member

@pascalberger pascalberger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another thing which needs to be updated for Azure Pipelines is that Codecov doesn't require a Token on Azure Pipelines here

@AdmiringWorm
Copy link
Member

@pascalberger actually, you still need to specify the token when running on Azure Pipelines.

The only CI's that don't need a token is Appveyor, Travis CI (coming in next version of codecov-exe) and circle-ci (not yet on the roadmap).

@vhatsura
Copy link
Contributor Author

vhatsura commented Aug 29, 2019

Hey, @gep13. How's going your work with updating upstreams? Is it any chance that you can review this PR in near month? It's long awaited feature from various cake add in and it would be nice to complete work with support of azure pipelines.

@gep13
Copy link
Member

gep13 commented Aug 29, 2019

@vhatsura we are getting there. You can see the progress in the Audit spreadsheet here:

https://github.com/cake-contrib/Home/blob/master/Audit.xlsx

Filter by the end column, with regard to what version of Cake.Recipe is being used. Without checking, I believe that there are 22 addins remaining to be updated, but this list started out at over 80 I think, so progress is being made 😄

In the meantime, there would be nothing to stop you using a custom build of Cake.Recipe with your changes in it, this would be good to work through any teething issues. I know that @AdmiringWorm is doing that on a couple of his addins, ahead of those changes getting pulled into Cake.Recipe for the next release.

@gep13
Copy link
Member

gep13 commented Sep 30, 2019

@vhatsura this has been rebased and merged here:

cc5ab0f

As a result, I am going to close this PR, however, your work has been correctly attributed in the commits merged into the develop branch.

@gep13
Copy link
Member

gep13 commented Sep 30, 2019

@vhatsura your changes have been merged, thanks for your contribution 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants